Repository navigation
[17/17] Opt-in FP16 strong typing for DLA - #2969
mgohil-png wants to merge 14 commits into
Conversation
|
Auto-sync is disabled for draft pull requests in this repository. Workflows must be run manually. Contributors can view more details about this message here. |
|
Warning Review limit reachedEnable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. Next included review available in 6 minutes. View limit details
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review.
|
| Layer / File(s) | Summary |
|---|---|
Opt-in pipeline and CLI wiring modelopt/onnx/graph_surgery/__main__.py, modelopt/onnx/graph_surgery/make_dla_compatible.py, CHANGELOG.rst |
Adds the default-disabled option to the CLI and transformation entry points. The enabled pipeline applies the FP16 transforms, performs shape inference, and validates the model. |
Normalization rewrite and opset handling modelopt/onnx/graph_surgery/dla_transforms/dla_force_fp16_activations.py, tests/unit/onnx/graph_surgery/dla_transforms/test_dla_force_fp16_activations.py |
Decomposes eligible last-axis L2 normalization and migrates supported quantizer models to standard opset 19. Tests cover normalization, axis mapping, nested graphs, and unsupported cases. |
FP16 conversion and FP32 preservation modelopt/onnx/graph_surgery/dla_transforms/dla_force_fp16_activations.py, tests/unit/onnx/graph_surgery/dla_transforms/test_dla_force_fp16_activations.py |
Validates FLOAT payloads, preserves required FP32 regions, converts eligible weights and activations, and inserts boundary casts. Tests cover controls, initializers, nested graphs, generated values, naming, and validation. |
Terminal projection splitting modelopt/onnx/graph_surgery/dla_transforms/dla_force_fp16_activations.py, tests/unit/onnx/graph_surgery/dla_transforms/test_dla_force_fp16_activations.py |
Splits matching terminal projection chains into chunks of at most 8192 output channels. Tests cover layouts, quantized weights, ownership conditions, legacy inputs, and output values. |
Priority:
Estimated code review effort: 4 (Complex) | ~60 minutes
Change: Feature
Sequence Diagram(s)
sequenceDiagram
participant CLI
participant run_graph_surgery
participant dla_make_dla_compatible
participant _apply_force_fp16_activations
participant ONNX_shape_inference
CLI->>run_graph_surgery: pass force_fp16_activations
run_graph_surgery->>dla_make_dla_compatible: forward force_fp16_activations
dla_make_dla_compatible->>_apply_force_fp16_activations: apply FP16 transform
dla_make_dla_compatible->>ONNX_shape_inference: infer shapes and validate
Suggested labels: waiting for feedback
Suggested reviewers: cjluo-nv, hthadicherla, vishalpandya1990
Merge Risk: 
Low · previous review 29354
-
The opt-in FP16 mode is a bounded change that leaves the default FP32 pipeline unchanged. One documented limitation remains: very large activations in the L2 normalization step can overflow in FP16 and silently produce zero outputs. Users should validate accuracy on the target runtime before enabling it.
Pre-merge checks: 
6
-
Full details: Docstring Coverage
-
Explanation
Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentation-only edits and deletions within surviving functions are included; unchanged diff context and fully deleted functions are excluded. Analyzed 43 functions across 4 files. (1 skipped: 1 unsupported.)
Full details: Security Anti-Patterns
-
Explanation
No security anti-patterns were introduced. The reviewed Python additions contain no torch.load(..., weights_only=False), numpy.load(..., allow_pickle=True), hardcoded trust_remote_code=True, external-input eval()/exec(), or # nosec comments. No pyproject.toml or requirements.txt files changed. The added onnx.load calls are test loading, not a prohibited pattern.
-
Finishing Touches: 💡 1
-
🛠️ Fix failing CI checks: 💡
-
- Commit to this branch
- Create a new PR
📝 Generate docstrings
-
- Commit to this branch
- Create a new PR
🧪 Generate unit tests (beta)
-
- Commit to this branch
- Create a new PR
-
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
Comment @coderabbitai help to get the list of available commands.
|
@coderabbitai full review |
|
35a0889 to
7915fb8
Compare
7915fb8 to
b3fc54a
Compare
|
46ad771 to
9964368
Compare
3736757 to
ed8f5d7
Compare
|
@coderabbitai full review |
✅ Action performed
|
|
@coderabbitai full review |
|
|
@coderabbitai full review |
✅ Action performed
|
|
@coderabbitai full review |
|
|
@coderabbitai full review |
|
|
@coderabbitai full review |
|
|
@coderabbitai full review |
✅ Action performed
|
Signed-off-by: mgohil <mgohil@nvidia.com>
Signed-off-by: mgohil <mgohil@nvidia.com>
Signed-off-by: mgohil <mgohil@nvidia.com>
Signed-off-by: mgohil <mgohil@nvidia.com>
Signed-off-by: mgohil <mgohil@nvidia.com>
Signed-off-by: mgohil <mgohil@nvidia.com>
Signed-off-by: mgohil <mgohil@nvidia.com>
Signed-off-by: mgohil <mgohil@nvidia.com>
Signed-off-by: mgohil <mgohil@nvidia.com>
Signed-off-by: mgohil <mgohil@nvidia.com>
Signed-off-by: mgohil <mgohil@nvidia.com>
Signed-off-by: mgohil <mgohil@nvidia.com>
Signed-off-by: mgohil <mgohil@nvidia.com>
Signed-off-by: mgohil <mgohil@nvidia.com>
|
@coderabbitai full review |
|
What does this PR do?
Type of change: new feature
Add opt-in
force_fp16_activationsto the public DLA API and CLI, retaining FP32 public boundaries and the default FP32 pipeline. Part 17/17 depends on part 16; merge from the bottom upward. This self-contained FP16 feature remains together under the approved source-size exception.Use ONNX type bindings to select supported FP16 operations and retain FP32 controls, generated values, structured outputs, nested captures and opaque functions where required. Preserve runtime-overridable initializer defaults. Eligible fixed FLOAT weights use the existing ModelOpt initializer converter; malformed or unavailable weights fail validation before mutation. Supported contrib quantizers and activation retain their original domains and imports. Standard quantizers upgrade only supported imports, migrating reduction axes in nested graphs when needed.
Scaled L2 normalization and terminal projection splitting retain native contracts, ownership and generated-name safety. Projection conversion requires fixed private weights and controls, validates supported layouts and groups, and splits eligible projections at the 8192-channel limit. Batch replacements preserve unrelated Constant payload objects. Legacy generated initializer declarations remain valid.
Terminal projection splitting additionally requires the Conv result shape
[1, channels, 1, 1]; nontrivial batch summation stays in the original chain. Public option documentation attaches QDQ mutual exclusion to the correct argument.FP16 L2 decomposition prevalidates collapsed axes with the shared normalization mapping before generating ReduceSum. A removed normalization axis refuses consistently with the native FP32 route, before earlier eligible normalization branches can be mutated.
Usage
The public conversion contract assumes finite FP32 inputs. Operations and controls requiring FP32 retain it.
Eligible scaled last-axis L2 decomposition uses axis -1 for non-five-dimensional inputs, so generated ReduceSum follows the last runtime dimension after leading-rank promotion. Five-dimensional inputs keep the shared collapse mapping and prevalidation.
When FP16 quantizers require opset 19, legacy equal Split nodes receive num_outputs, including nested graphs. Empty optional split-input placeholders are removed when adding that attribute so ONNX Runtime recognizes the absent input. Explicit sizes and existing num_outputs remain unchanged. Six legacy equal-split cases fail before the fix; twelve no-Split, explicit-size and modern-schema controls pass. All 101 FP16 tests pass with graph checking and CPU output comparisons.
The public API and CLI help now document the existing FP16 L2 arithmetic limitation. Finite FP32 inputs can overflow squared terms or their sum, and small squares can underflow; the denominator retains its existing 2**-14 floor. Users should validate on the target runtime and leave FP16 mode disabled when the intermediates need a wider range. Numerical rescaling is intentionally deferred to preserve reference graph and weight parity. Four strict-FP16 counterexamples reproduce the limitation; the normalization algorithm is unchanged.
Removed test_excluded_initializers_are_restored_when_conversion_fails because payload validation rejects its fixture before converter rollback is reached. The existing malformed-late-weight test retains the stronger whole-model unchanged-state assertion. The FP16 normalization override also forwards the registered rank context to the shared rank stage; eight existing low-rank execution cases fail without that forwarding and eight FP32 controls remain valid. All 101 FP16 tests pass. The arithmetic recipe and the documented numerical limitation are unchanged.
Internal FP16-eligible readers of a public Squeeze output now share a private FP16 cast. The public output stays FP32; native controls and existing FP16 Cast readers keep their original inputs. Two arithmetic execution cases fail without the fix; two native-control cases remain valid. All 101 FP16 tests pass. The normalization recipe and its documented limitation are unchanged. A Windows CPU sample with 512 readers and a retained 64 MiB payload uses one shared public-output cast, takes 0.013661 seconds and adds 2.1367 MiB sampled peak RSS. Payload bytes/identity, graph checking and CPU values are verified. Single sample with 2 ms polling.
Final FP16 validation now runs ONNX inference with check_type=True and strict_mode=True, then structural checking. These match the strict typed-inference settings used by full_check, without repeating inference on a copied model. Both public-API regressions fail before the fix and now reject malformed Resize scale types before creating or replacing output files; all 91 FP16 tests pass. For 512 readers and a retained 64 MiB payload, one sequential Windows CPU comparison takes 0.655895 seconds/+261.6914 MiB sampled peak RSS before and 0.670173 seconds/+261.5703 MiB after; complete graph hashes and CPU values match. Repeating full_check instead needs +325.3555 MiB in the same sample. Single samples, 2 ms polling; GPU execution was not measured. The FP16 arithmetic module is unchanged.
Public Cast-to-FLOAT outputs now share the existing Squeeze boundary handling: arithmetic readers receive a shared private FP16 view, while the public FLOAT value, explicit Cast readers and FP32 control inputs retain their requested precision. Six INT32/INT64 regressions fail strict typed checking before the fix; four control cases retain existing behavior. All 101 FP16 tests pass, including the actual save/reload API, ReferenceEvaluator rounding checks and ORT CPU execution. ORT CPU promotes FP16 arithmetic, so rounding-sensitive values are checked with ReferenceEvaluator and representable values with ORT. One Windows CPU sample rewrites 512 readers with a retained 64 MiB payload in 0.013800 seconds and +2.1953 MiB sampled peak RSS (2 ms polling); payload bytes and identity are preserved, a single private view is shared, and every CPU output is checked. GPU execution was not measured. L2 arithmetic is unchanged.
Testing
Windows CPU: 2049 passed in 360.69 seconds across the complete restacked graph-surgery and compatibility suite with Python 3.11.18, ONNX 1.23.2 and ONNX Runtime 1.22.0. The public DLA API uses direct ONNX saving; existing initializer conversion and unrelated surgeries are preserved. Commits are signed and all commit hooks pass with the equivalent Linux mypy platform setting. GPU execution, DLA compilation and the complete Linux suite were not run locally.
Before your PR is "Ready for review"
Additional Information
Base branch:
dla/review-16-dla-public-integration-and-reshape-cleanup.Merge order: PR1 (merged); Part 2 (withdrawn); then Part 3/17 → Part 4/17 → Part 5/17 → Part 6/17 → Part 7/17 → Part 8/17 → Part 9/17 → Part 10/17 → Part 11/17 → Part 12/17 → Part 13/17 → Part 14/17 → Part 15/17 → Part 16/17 → Part 17/17.